Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a conversion mechanism between NLPModel to OptimizationProblem #792

Merged
merged 19 commits into from
Aug 19, 2024
Merged

Add a conversion mechanism between NLPModel to OptimizationProblem #792

merged 19 commits into from
Aug 19, 2024

Conversation

alonsoC1s
Copy link
Contributor

@alonsoC1s alonsoC1s commented Aug 7, 2024 •
edited
Loading

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

Would close #790 by adding a new constructor OptimizationProblem(model::AbstractNLPModel, ...) that makes the problem defined as an NLPModel available as an OptimizationProblem and thus, makes it available to the rest of the SciML ecosystem

So far the appropriate tests are still missing and the public API documentation hasn't been updated

@alonsoC1s
Copy link
Contributor Author

@Vaibhavdixit02 I'm working on the tests and I ran into an issue. Since OptimizationFunction doesn't define all fields (e.g. cons_h), it stays as missing and throws an error deeper down in the solvers. Those fields are not directly available from the NLPModel API. How should I compute them? Re-introduce the ADType?

@Vaibhavdixit02
Copy link
Member

Can you show the code you ran and the error?

@alonsoC1s
Copy link
Contributor Author

I was manually running through the tests

    hs5f(u, p) = sin(u[1] + u[2]) + (u[1] - u[2])^2 - (3 / 2) * u[1] + (5 / 2)u[2] + 1
    f = Optimization.OptimizationFunction((u, p) -> hs5f(u, p), Optimization.AutoForwardDiff())
    lb = [-1.5; -3]
    ub = [4; 3]
    u0 = [0.0; 0.0]
    oprob = Optimization.OptimizationProblem(
        f, u0, lb = lb, ub = ub, sense = Optimization.MinSense)
    nlpmo = NLPModelsTest.HS5()

    converted = OptimizationNLPModels.OptimizationProblem(nlpmo)

solve(converted, Optim.IPNewton())

And the full error is

julia> solve(converted, Optim.IPNewton())
ERROR: MethodError: objects of type Nothing are not callable
Stacktrace:
  [1] (::OptimizationOptimJL.var"#29#36"{OptimizationCache{…}})(h::Matrix{Float64}, θ::Vector{Float64}, λ::Vector{Float64})
    @ OptimizationOptimJL ~/.julia/packages/OptimizationOptimJL/hDX5k/src/OptimizationOptimJL.jl:418
  [2] hessianI!(h::Matrix{…}, x::Vector{…}, constraints::TwiceDifferentiableConstraints{…}, λcI::Vector{…}, μ::Int64)
    @ Optim ~/.julia/packages/Optim/ZhuZN/src/multivariate/solvers/constrained/ipnewton/interior.jl:462
  [3] hessianI(x::Vector{…}, constraints::TwiceDifferentiableConstraints{…}, λcI::Vector{…}, μ::Int64)
    @ Optim ~/.julia/packages/Optim/ZhuZN/src/multivariate/solvers/constrained/ipnewton/interior.jl:476
  [4] initial_state(method::IPNewton{…}, options::Optim.Options{…}, d::TwiceDifferentiable{…}, constraints::TwiceDifferentiableConstraints{…}, initial_x::Vector{…})
    @ Optim ~/.julia/packages/Optim/ZhuZN/src/multivariate/solvers/constrained/ipnewton/ipnewton.jl:167
  [5] optimize(d::TwiceDifferentiable{…}, constraints::TwiceDifferentiableConstraints{…}, initial_x::Vector{…}, method::IPNewton{…}, options::Optim.Options{…})
    @ Optim ~/.julia/packages/Optim/ZhuZN/src/multivariate/solvers/constrained/ipnewton/interior.jl:229
  [6] __solve(cache::OptimizationCache{…})
    @ OptimizationOptimJL ~/.julia/packages/OptimizationOptimJL/hDX5k/src/OptimizationOptimJL.jl:443
  [7] solve!(cache::OptimizationCache{…})
    @ SciMLBase ~/.julia/packages/SciMLBase/vhP5T/src/solve.jl:188
  [8] solve(::SciMLBase.OptimizationProblem{…}, ::IPNewton{…}; kwargs::@Kwargs{})
    @ SciMLBase ~/.julia/packages/SciMLBase/vhP5T/src/solve.jl:96
  [9] solve(::SciMLBase.OptimizationProblem{…}, ::IPNewton{…})
    @ SciMLBase ~/.julia/packages/SciMLBase/vhP5T/src/solve.jl:93
 [10] top-level scope
    @ REPL[102]:1
Some type information was truncated. Use `show(err)` to see complete types.

The actual error is not that informative, the line where it breaks is

        cache.f.cons_h(res, θ)

@Vaibhavdixit02
Copy link
Member

Okay so the issue is because IPNewton from Optim expects cons_h to be defined as you said, the conversion doesn't run instantiate_function so the fields are not populated by us either. So this is not really an issue but basically incompatibility of the cutest models with IPNewton.

You can add a adtype argument to the conversion so it can populate the missing fields using the AD backend choice.

Alternatively just use a different optimizer here, Ipopt should be able to work with the lagrangian directly.

@alonsoC1s alonsoC1s marked this pull request as ready for review August 14, 2024 15:50
@Vaibhavdixit02
Copy link
Member

@alonsoC1s please pull the branch before pushing the next commit since I made some changes above

@alonsoC1s
Copy link
Contributor Author

@Vaibhavdixit02 I tried using LBFGS as in the changes you made but I was getting some errors that looked unrelated. That's why I had changed it back. Let me investigate further

@Vaibhavdixit02
Copy link
Member

Yeah sorry about that, this should work now

Vaibhavdixit02
Vaibhavdixit02 reviewed Aug 15, 2024
View reviewed changes
Copy link
Member

@Vaibhavdixit02 Vaibhavdixit02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's missing the new packages in the docs environment can you add that, and also run the formatter. We should be done then!

@Vaibhavdixit02
Copy link
Member

@alonsoC1s the documentation is failing and looks like a real failure can you take a look?

@Vaibhavdixit02
Copy link
Member

I think this should fix it

@Vaibhavdixit02 Vaibhavdixit02 merged commit 990b907 into SciML:master Aug 19, 2024
23 of 26 checks passed
@amontoison
Copy link

Great work!!!
We should maybe add an entry about it in the README.md?

cc @tmigot

@tmigot
Copy link

tmigot commented Aug 19, 2024

That's cool! Thanks @alonsoC1s @Vaibhavdixit02
We should probably think of a wrapper the other way around too

@Vaibhavdixit02
Copy link
Member

@amontoison yeah I'll update the readme it's a bit outdated now

@tmigot sure it shouldn't be a lot of work to do it the other way around too. Just to clarify our main motivation for this PR was to access Cutest benchmarks with Optimization.jl

@alonsoC1s
Copy link
Contributor Author

@tmigot That sounds interesting and useful. I think Optimization.jl wraps more optimizers, so having the mechanism to switch between could be nice

@alonsoC1s alonsoC1s deleted the wrap_nlpmodel branch August 20, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Vaibhavdixit02 Vaibhavdixit02 Vaibhavdixit02 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adding a thin wrapper to NLPModels
4 participants
@alonsoC1s @Vaibhavdixit02 @amontoison @tmigot